Allow setting concurrency for pipelined flush and resolveLocks #1494
Conversation
Signed-off-by: ekexium <[email protected]>
Signed-off-by: ekexium <[email protected]>
Signed-off-by: ekexium <[email protected]>
ti-chi-bot
bot
added
dco-signoff: yes
Signed-off-by: ekexium <[email protected]>
ekexium
force-pushed
the
config-flush-concurrency
branch
from
9935299 to
d3adfc2
Compare
Signed-off-by: ekexium <[email protected]>
ekexium
force-pushed
the
config-flush-concurrency
branch
from
ecf9e34 to
6b75b34
Compare
ti-chi-bot
bot
added
needs-1-more-lgtm
|
@you06 PTAL |
examples/gcworker/go.mod
Outdated
| @@ -1,6 +1,6 @@ | |||
| module gcworker | |||
|
|
|||
| go 1.23 | |||
| go 1.23.2 | |||
There was a problem hiding this comment.
Is it an auto update from IDE? I think go1.23 is enough.
There was a problem hiding this comment.
I forget the reason to do so, probably because of compiling or CI stuff. I will revert it
| PipelinedMemDB bool | ||
| TxnScope string | ||
| StartTS *uint64 | ||
| PipelinedTxn PipelinedTxnOptions |
There was a problem hiding this comment.
Can you also modify the initialization of TxnOptions here to use manually specified default properties?
Line 364 in f2025c1
There was a problem hiding this comment.
Manually sepecifying default values seems a bit strange to me. These fields must be correctly specified by upper layers if they are used. And they have a valid range check when used.
I add validity checks for them before usage. PTAL
| @@ -481,7 +480,7 @@ func (c *twoPhaseCommitter) resolveFlushedLocks(bo *retry.Backoffer, start, end | |||
| fmt.Sprintf("pipelined-dml-%s", status), | |||
| fmt.Sprintf("pipelined-dml-%s-%d", status, c.startTS), | |||
| c.store, | |||
| RESOLVE_CONCURRENCY, | |||
| c.txn.pipelinedResolveLockConcurrency, | |||
There was a problem hiding this comment.
The throttle flow control works when flushing.
And the resolve-lock phase can also consume many resources, and it may also needs to be controlled by throttle, what about adding a TODO here.
There was a problem hiding this comment.
Throttling the resolve lock goroutine is in our current plan I think. For now we rely on the concurrency to constrain it. If we want a better control of its resource usage we might need to consider a more comprehensive mechanism, e.g. one that helps reduce the overhead of resolving locks of multiple p-txns running in parallel.
Co-authored-by: you06 <[email protected]>
Signed-off-by: ekexium <[email protected]>
…go into config-flush-concurrency
Signed-off-by: ekexium <[email protected]>
ti-chi-bot
bot
added
size/XL
Signed-off-by: ekexium <[email protected]>
Signed-off-by: ekexium <[email protected]>
ekexium
force-pushed
the
config-flush-concurrency
branch
from
4dc212d to
4af7b8b
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cfzjywxk, you06 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
For tidb variable
tidb_pipelined_dml_resource_policy